Skip to content
This repository was archived by the owner on Jul 22, 2025. It is now read-only.

Conversation

@SamSaffron
Copy link
Member

  1. Legacy reasoning models (o1-preview / o1-mini) do not support developer or system messages, do not use them.
  2. LLM editor form not showing all provider params due to missing remap

1. Legacy reasoning models (o1-preview / o1-mini) do not support developer or system messages, do not use them.
2. LLM editor form not showing all provider params due to missing remap
@tgxworld
Copy link
Contributor

@SamSaffron I'm not familiar with the plugin and its constraints but I'm curious why we opted not to test the fix here.

@SamSaffron
Copy link
Member Author

The UI needs a system test, I agree with that, but the the backend is pretty fluid ad open ai break stuff all the time, so I think the backend fix is OK without a test.

will see if I can add 1 system test for llm editing... we have none I guess.

Copy link
Contributor

@tgxworld tgxworld left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approving first as the fix looks good to me.

@SamSaffron
Copy link
Member Author

@tgxworld added a system test, oddly enough we covered a lot here just missed the edit case.

@tgxworld
Copy link
Contributor

@tgxworld added a system test, oddly enough we covered a lot here just missed the edit case.

Looks good to me 👍

@SamSaffron SamSaffron merged commit 84e791a into main Feb 24, 2025
6 checks passed
@SamSaffron SamSaffron deleted the fixes-o1 branch February 24, 2025 05:38
keegangeorge pushed a commit that referenced this pull request Feb 27, 2025
)

* FIX: legacy reasoning models not working, missing provider params

1. Legacy reasoning models (o1-preview / o1-mini) do not support developer or system messages, do not use them.
2. LLM editor form not showing all provider params due to missing remap

* add system test
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Development

Successfully merging this pull request may close these issues.

3 participants